Replace Platform.IsMono with actual OS checks #356
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some cases of Platform.IsMono are workarounds for bugs in older versions of mono. Those can probably be removed, but for safety's sake, we'll leave them alone for now. Other cases are clearly trying to check the platform, to run Linux-only or Windows-only code. Those cases should be replaced by Platform.IsUnix or Platform.IsWindows as appropriate, because nearly everywhere that Chorus is being run on Linux these days, it's running under dotnet rather than mono, and the Platform.IsMono check returns false.
|
@hahn-kev - This bug is "in the wild" in LfMerge. If LfMerge ever encounters a merge conflict between two .txt files, it will try to run Windows-only code and promptly error out, resulting in a Mercurial rollback because the merge conflict wasn't handled. Thankfully it's extremely unlikely that anyone will check .txt files into their FieldWorks project, let alone end up with a merge conflict in those files. However, I want to prioritize reviewing and merging this bugfix so that I can push a new LfMerge release that won't try to run Windows-only code. |
hahn-kev
requested changes
Nov 5, 2024
hahn-kev
approved these changes
Nov 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #355.
Some cases of Platform.IsMono are workarounds for bugs in older versions of mono. Those can probably be removed, but for safety's sake, we'll leave them alone for now.
Other cases are clearly trying to check the platform, to run Linux-only or Windows-only code. Those cases should be replaced by Platform.IsUnix or Platform.IsWindows as appropriate, because nearly everywhere that Chorus is being run on Linux these days, it's running under dotnet rather than mono, and the Platform.IsMono check returns false.
This change is